Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Advantage performance of Fetch/FetchAsync #1121

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

BFuerchau
Copy link

Now, i have found the tests within the sources and have corrected the ReadObject-Function with adding of DBNull.Value.
Some tests fails because of my current environment with parallel installations of 3 firebird versions.
E.g.: FirebirdSql.Data.Common.IscException : Database is probably already opened by another engine instance in another Windows session

Also i have understand how to use branch in the git.

The most advantage is the less object creation and less GC work during Fetch().
In case of FetchAsync(), which is always slower, the advantage may be less.

Change _rows-Queue from DbValue[] to object[]
Store only object[]-Array
Use single DbValue[]-Array

Advantage:
Less object creation, less Garbagecollector, performance increase up to 10%
matching override to gdsstatement version 10
@BFuerchau
Copy link
Author

How can i identify the failing test in the test source?
I'm running all tests, except some with missing plugins, fine. Also the embedded tests running now, if i execeute them test by test,

@cincuranet
Copy link
Member

Not sure what you're asking. But you see the name of failing test(s) and with that you can check the code easily.

@BFuerchau
Copy link
Author

Some tests fails:
CompleteDatabaseInfoTest
FbDatabaseInfo.GetMaxMemory/GetCurrentMemory is of type int, but if you have more than 2 GB assigned, you get an overflow exception, i have assigned 8GB to FB.

All tests with northwind fails when i use FB-Server 3.0.10, may be depends on my environment.
With FB 4.0 all works fine.
Finally i have only fails with authority tests because a plugin is missing, and 1 migration test, because the windows environment adds "\r" to the command source in the sourcefile where the build result contains only "\n". When i remove the "\r", test runs ok.

But when i look at the logs in the attachd checks i find only one error:

  1. Error : FirebirdSql.Data.FirebirdClient.Tests.FbArrayTests(Default,True,Required).BigArrayTest
    FirebirdSql.Data.FirebirdClient.FbException : Unable to complete network request to host "localhost".
    I have checked these tests with FB 3.0 and FB 4.0 and both works fine.
    And i don't understand why connection problems should have to do with my changes in fetch.

@BFuerchau
Copy link
Author

What is the further procedure now?
I cannot reproduce the above error in the test.
The test is positive for me with FB 3.0 and FB 4.0.

@BFuerchau
Copy link
Author

BFuerchau commented Aug 13, 2023

I have checked once more the error from the test (BigArrayTest):
https://github.com/FirebirdSQL/NETProvider/actions/runs/5545201297/job/15020409952?pr=1121
The test fails during create database.
I have called this test separately and get no error.
So what should i do now?

@cincuranet
Copy link
Member

Could you please modify the ReadRow directly, so it's easier to see the diff. We don't have to keep the original version.

@BFuerchau
Copy link
Author

Are my changes rolled back?
Should i do a complete new request?

@cincuranet
Copy link
Member

You should modify this PR, preferably.

Simplified to ReadRow(), ReadRowAsync()
Simplified to ReadRow(), ReadRowAsync()
@BFuerchau
Copy link
Author

Ok, i have updated the Patch-1 now (i was on the wrong patch before).
Is this now enough? I can't make a new pull request.
Is the current pull request updated?

Sorry for my questions.

@BFuerchau
Copy link
Author

Why haven't any of my requests been taken into account?
Not even the choppy suggestion for FbDataRader?

@MartinKoeditz
Copy link

I think this is a interesting patch regarding performance. Would like to see it.

@BFuerchau
Copy link
Author

I think this is a interesting patch regarding performance. Would like to see it.

First changes in FbDataReader:
#1122

Second changes in GdsStatements (concerning garbage and object creation):
#1121

Complete fork, based on 9.1.0:
https://github.com/BFuerchau/NETProvider

What do you like to know more specific?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants